Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change IsolatedStorageFile path for mobile #83380

Merged
merged 15 commits into from
May 11, 2023

Conversation

mkhamoyan
Copy link
Contributor

@mkhamoyan mkhamoyan commented Mar 14, 2023

Fix GetRandomDirectory for mobile, if there is no existing random directory create the path without hashes.

In legacy Xamarin we didn't have a random directory inside of the isolated storage root for each app, we tried to preserve that in #75541 but the fix wasn't complete enough and we still created random directories when not using the Roaming scope.

Since we shipped that behavior as part of .NET 7 we can't change this now or upgraded apps wouldn't find their files anymore. We need to look for an existing random directory first before using the plain root directory.

Example for Android:
Before this change path is /data/user/0/{packageName}/files/.isolated-storage/{hash}/{hash}/AppFiles/
After the change path will be /data/user/0/{packageName}/files/.config/.isolated-storage/

Example for iOS:
Before this change path is /Users/userName/{packageName}/Documents/.isolated-storage/{hash}/{hash}/AppFiles/
After the change path will be /Users/userName/{packageName}/Documents/.config/.isolated-storage/

Fixes #83231, #83384

@ghost
Copy link

ghost commented Mar 14, 2023

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

In case of mobile platforms return GetDataDirectory.
GetRootDirectory (in case of machine root directory and user root directory see https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.IsolatedStorage/src/System/IO/IsolatedStorage/Helper.cs#L28 ) appends hashes to the path like
/data/user/0/{packageName}/files/.isolated-storage/{hash}/{hash}/AppFiles/

Fixes #83231

Author: mkhamoyan
Assignees: -
Labels:

area-System.IO

Milestone: -

@ghost
Copy link

ghost commented Mar 14, 2023

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

In case of mobile platforms return GetDataDirectory.
GetRootDirectory (in case of machine root directory and user root directory see https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.IsolatedStorage/src/System/IO/IsolatedStorage/Helper.cs#L28 ) appends hashes to the path like
/data/user/0/{packageName}/files/.isolated-storage/{hash}/{hash}/AppFiles/

Fixes #83231

Author: mkhamoyan
Assignees: mkhamoyan
Labels:

area-System.IO, os-android

Milestone: -

@mkhamoyan mkhamoyan added the os-ios Apple iOS label Mar 14, 2023
@ghost
Copy link

ghost commented Mar 14, 2023

Tagging subscribers to 'os-ios': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

In case of mobile platforms return GetDataDirectory.
GetRootDirectory (in case of machine root directory and user root directory see https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.IsolatedStorage/src/System/IO/IsolatedStorage/Helper.cs#L28 ) appends hashes to the path like
/data/user/0/{packageName}/files/.isolated-storage/{hash}/{hash}/AppFiles/

Fixes #83231

Author: mkhamoyan
Assignees: mkhamoyan
Labels:

area-System.IO, os-android, os-ios

Milestone: -

@mkhamoyan

This comment was marked as outdated.

@azure-pipelines

This comment was marked as outdated.

@mkhamoyan

This comment was marked as outdated.

@azure-pipelines

This comment was marked as outdated.

@mkhamoyan

This comment was marked as outdated.

@azure-pipelines

This comment was marked as outdated.

@mkhamoyan

This comment was marked as outdated.

@azure-pipelines

This comment was marked as outdated.

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-android

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-ioslike

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan mkhamoyan requested a review from akoeplinger March 30, 2023 09:32
@mkhamoyan
Copy link
Contributor Author

@akoeplinger could you please review last changes?

Copy link
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me apart from one case on Android

{
specialFolder =
IsMachine(scope) ? Environment.SpecialFolder.CommonApplicationData :
IsRoaming(scope) ? Environment.SpecialFolder.LocalApplicationData:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to change this case on Android since we had the breaking change of Environment.SpecialFolder.LocalApplicationData between Xamarin and dotnet:

Xamarin /data/user/0/com.companyname.testandroidxamarin/files/.local/share
dotnet /data/user/0/com.companyname.TestMauiNet7/files

You can use hardcoded SpecialFolder.UserProfile + ".local/share" for that when OperatingSystem.IsAndroid()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general I'd recommend creating a sample app that uses all possible combinations for IsolatedStorageFile scopes and then comparing each computed path between Xamarin and dotnet to make sure they match

// In legacy Xamarin for Roaming Scope we were using Environment.SpecialFolder.LocalApplicationData
// In .Net 7 for Roaming Scope we are using Environment.SpecialFolder.ApplicationData
// e.g. .Net 7 path = /data/user/0/{packageName}/files/.isolated-storage/{hash}/{hash}/AppFiles/
// e.g. Xamarin path = /data/user/0/{packageName}/files/.config/.isolated-storage"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these example paths are from Android, I'd also add iOS examples to make it clearer

internal static string GetDataDirectory(IsolatedStorageScope scope)
{
// This is the relevant special folder for the given scope plus IsolatedStorageDirectoryName.
// It is meant to replicate the behavior of the VM ComIsolatedStorage::GetRootDir().
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment isn't meaningful for us, I'd remove it.

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-ioslike

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-android

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-ioslike

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-android

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-android

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-ioslike

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

Failures are not related.

@mkhamoyan mkhamoyan merged commit 1ed1206 into dotnet:main May 11, 2023
@mkhamoyan mkhamoyan deleted the 83231_isolated_storage_path branch May 11, 2023 09:33
@steveisok
Copy link
Member

@akoeplinger @mkhamoyan should we try to backport this to 7?

@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android: IsolatedStorageFile path changes
3 participants